-
Notifications
You must be signed in to change notification settings - Fork 592
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
storage: fix credentials issue. #164
Conversation
Connection.prototype.getCredentials = function(callback) { | ||
var that = this; | ||
if (this.credentials) { | ||
process.nextTick(function() { |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
15484d5
to
0de5a0d
Compare
} | ||
this.fetchToken(function(err) { | ||
if (err) { | ||
callback(err); |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
c47c526
to
25a6b06
Compare
Previously, if a user wanted to generate a signed URL, there was no guarantee a credentials object would have already been generated. By itself, generating a signed URL doesn't require a network connection, thus the uncertainty that we would have already fetched a token. This change requires a valid credentials object to exist before returning the `credentials` object.
25a6b06
to
bb4cd17
Compare
Sorry, I'm not familiar with signed URLs. Is there anyone else to review it @silvolu, from Storage team? |
Np, this is actually mostly an API change - adding the Currently, we wait until an API request to fetch a token, which makes sense. But to create a signed URL, we don't actually have to make an API call. So, we never get a credentials object (unless the user provided it themselves), which is why I added this method. It will return the credentials if they're available, otherwise it will go build it for us by making a fetchToken call. |
@@ -95,9 +95,10 @@ module.exports.Token = Token; | |||
function Connection(opts) { | |||
events.EventEmitter.call(this); | |||
|
|||
this.opts = opts || {}; | |||
opts = opts || {}; |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
storage: fix credentials issue.
* updated CHANGELOG.md [ci skip] * updated package.json [ci skip] * updated samples/package.json [ci skip]
* build: check dead links on Kokoro * recursive crawl local links * annotate Project#exists with @method * npm run fix
[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [jsdoc](https://github.com/jsdoc/jsdoc) | [`^3.6.6` -> `^4.0.0`](https://renovatebot.com/diffs/npm/jsdoc/3.6.11/4.0.0) | [![age](https://badges.renovateapi.com/packages/npm/jsdoc/4.0.0/age-slim)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://badges.renovateapi.com/packages/npm/jsdoc/4.0.0/adoption-slim)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://badges.renovateapi.com/packages/npm/jsdoc/4.0.0/compatibility-slim/3.6.11)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://badges.renovateapi.com/packages/npm/jsdoc/4.0.0/confidence-slim/3.6.11)](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>jsdoc/jsdoc</summary> ### [`v4.0.0`](https://github.com/jsdoc/jsdoc/blob/HEAD/CHANGES.md#​400-November-2022) [Compare Source](https://github.com/jsdoc/jsdoc/compare/3.6.11...084218523a7d69fec14a852ce680f374f526af28) - JSDoc releases now use [semantic versioning](https://semver.org/). If JSDoc makes backwards-incompatible changes in the future, the major version will be incremented. - JSDoc no longer uses the [`taffydb`](https://taffydb.com/) package. If your JSDoc template or plugin uses the `taffydb` package, see the [instructions for replacing `taffydb` with `@jsdoc/salty`](https://github.com/jsdoc/jsdoc/tree/main/packages/jsdoc-salty#use-salty-in-a-jsdoc-template). - JSDoc now supports Node.js 12.0.0 and later. </details> --- ### Configuration 📅 **Schedule**: Branch creation - "after 9am and before 3pm" (UTC), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://app.renovatebot.com/dashboard#github/googleapis/nodejs-contact-center-insights). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNC4xNy4xIiwidXBkYXRlZEluVmVyIjoiMzQuMTcuMSJ9-->
- [ ] Regenerate this pull request now. PiperOrigin-RevId: 408439482 Source-Link: googleapis/googleapis@b9f6184 Source-Link: googleapis/googleapis-gen@eb888bc Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiZWI4ODhiYzIxNGVmYzdiZjQzYmY0NjM0YjQ3MDI1NDU2NWE2NTlhNSJ9
🤖 I have created a release \*beep\* \*boop\* --- ## [2.13.0](https://www.github.com/googleapis/nodejs-dialogflow-cx/compare/v2.12.1...v2.13.0) (2021-08-17) ### Features * add advanced settings for agent level feat: add rollout config, state and failure reason for experiment feat: add insights export settings for security setting feat: add language code for streaming recognition result and flow versions for query p... ([#156](https://www.github.com/googleapis/nodejs-dialogflow-cx/issues/156)) ([80d2f94](https://www.github.com/googleapis/nodejs-dialogflow-cx/commit/80d2f94bcfaa65ea442ee4f8133d850035be04ed)) * added support for DLP templates; expose `Locations` service to get/list avaliable locations of Dialogflow products ([#162](https://www.github.com/googleapis/nodejs-dialogflow-cx/issues/162)) ([b77bde9](https://www.github.com/googleapis/nodejs-dialogflow-cx/commit/b77bde9893cde0c9e766b8e844fa4cca61efb5aa)) ### Bug Fixes * **build:** migrate to using main branch ([#164](https://www.github.com/googleapis/nodejs-dialogflow-cx/issues/164)) ([092d69d](https://www.github.com/googleapis/nodejs-dialogflow-cx/commit/092d69d76716297aff6a671200088d5740b1fd47)) * **deps:** google-gax v2.24.1 ([#165](https://www.github.com/googleapis/nodejs-dialogflow-cx/issues/165)) ([6261c37](https://www.github.com/googleapis/nodejs-dialogflow-cx/commit/6261c37019b3ce94a9876bfa4634837132691888)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
samples: pull in latest typeless bot, clean up some comments Source-Link: https://github.com/googleapis/synthtool/commit/0a68e568b6911b60bb6fd452eba4848b176031d8 Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-nodejs:latest@sha256:5b05f26103855c3a15433141389c478d1d3fe088fb5d4e3217c4793f6b3f245e
- [ ] Regenerate this pull request now. Committer: @summer-ji-eng PiperOrigin-RevId: 434859890 Source-Link: googleapis/googleapis@bc2432d Source-Link: googleapis/googleapis-gen@930b673 Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiOTMwYjY3MzEwM2U5MjUyM2Y4Y2ZlZDM4ZGVjZDdkM2FmYWU4ZWJlNyJ9
This PR was generated using Autosynth. 🌈 Synth log will be available here: https://source.cloud.google.com/results/invocations/13e8b547-7af0-436b-b85e-2c1942f8f36a/targets Source-Link: googleapis/synthtool@be74d3e
🤖 I have created a release \*beep\* \*boop\* --- ### [1.7.4](https://www.github.com/googleapis/nodejs-ai-platform/compare/v1.7.3...v1.7.4) (2021-07-12) ### Bug Fixes * **deps:** google-gax v2.17.1 ([#164](https://www.github.com/googleapis/nodejs-ai-platform/issues/164)) ([1ec11fc](https://www.github.com/googleapis/nodejs-ai-platform/commit/1ec11fc4fac373ee1a28ac10676fbfe9fc5535d1)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Previously, if a user wanted to generate a signed URL, there was no guarantee a credentials object would have already been generated. By itself, generating a signed URL doesn't require a network connection, thus the uncertainty that we would have already fetched a token. This change requires a valid credentials object to exist before returning the
credentials
object.This also brings up an issue. We need to go through the code to catch any "maybe async, maybe not" functions, and add in a(#166)process.nextTick
where necessary.